Skip to content

Fix stale query timeout guards interrupting unrelated queries#217

Merged
penberg merged 1 commit intomainfrom
fix-iterator-timeout-guard-leak
Apr 15, 2026
Merged

Fix stale query timeout guards interrupting unrelated queries#217
penberg merged 1 commit intomainfrom
fix-iterator-timeout-guard-leak

Conversation

@penberg
Copy link
Copy Markdown
Contributor

@penberg penberg commented Apr 10, 2026

The TimeoutGuard held by RowsIterator was only released when the JS garbage collector collected the native object. In a tight loop of stmt.all() calls (which uses iterate() internally), exhausted iterators would pile up unreachable but not yet GC'd. When their timeout deadline fired — potentially hundreds of milliseconds after the query finished — conn.interrupt() would kill whatever unrelated query happened to be running at that moment, producing spurious SQLITE_INTERRUPT errors well below the configured timeout.

Fix: eagerly release the TimeoutGuard as soon as the iterator is exhausted (next() returns None), encounters an error, or is explicitly closed via the iterator return() protocol (break/throw in for...of).

  • Rust: change _timeout_guard: Option<TimeoutGuard> to Mutex<Option<TimeoutGuard>> so close() and next() can both release the guard through a shared reference. Expose close() via napi.
  • promise.js / compat.js: wrap the native iterator with an idempotent close helper and implement return() for early-termination cleanup.
  • Add integration test that reproduces the original bug: 100 sequential stmt.all() calls with a 500ms timeout, each finishing in ~115ms, which reliably triggered stale-guard interrupts before this fix.

@penberg penberg force-pushed the fix-iterator-timeout-guard-leak branch 5 times, most recently from fce8d17 to 657f49e Compare April 15, 2026 11:43
The TimeoutGuard held by RowsIterator was only released when the JS
garbage collector collected the native object. In a tight loop of
stmt.all() calls (which uses iterate() internally), exhausted iterators
would pile up unreachable but not yet GC'd. When their timeout deadline
fired — potentially hundreds of milliseconds after the query finished —
conn.interrupt() would kill whatever unrelated query happened to be
running at that moment, producing spurious SQLITE_INTERRUPT errors well
below the configured timeout.

Fix: eagerly release the TimeoutGuard as soon as the iterator is
exhausted (next() returns None), encounters an error, or is explicitly
closed via the iterator return() protocol (break/throw in for...of).

- Rust: change `_timeout_guard: Option<TimeoutGuard>` to
  `Mutex<Option<TimeoutGuard>>` so close() and next() can both release
  the guard through a shared reference. Expose close() via napi.
- promise.js / compat.js: wrap the native iterator with an idempotent
  close helper and implement return() for early-termination cleanup.
- Add integration test that reproduces the original bug: 100 sequential
  stmt.all() calls with a 500ms timeout, each finishing in ~115ms,
  which reliably triggered stale-guard interrupts before this fix.
@penberg penberg force-pushed the fix-iterator-timeout-guard-leak branch from 657f49e to 5596b09 Compare April 15, 2026 12:09
@penberg penberg merged commit 0a3d2bd into main Apr 15, 2026
16 checks passed
@penberg penberg deleted the fix-iterator-timeout-guard-leak branch April 15, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant